PHOENIX-7786 Improved handling for empty files in Replication Log Processor#2392
PHOENIX-7786 Improved handling for empty files in Replication Log Processor#2392Himanshu-g81 wants to merge 6 commits intoapache:PHOENIX-7562-feature-newfrom
Conversation
| // If trailer is missing or corrupt, create reader without trailer validation | ||
| LOG.warn("Invalid Trailer for file {}", filePath, invalidLogTrailerException); | ||
| logFileReaderContext.setValidateTrailer(false); | ||
| logFileReader.init(logFileReaderContext); |
There was a problem hiding this comment.
There is still a leak here because the socket is created in the init call. https://github.com/apache/phoenix/blob/PHOENIX-7562-feature-new/phoenix-core-server/src/main/java/org/apache/phoenix/replication/log/LogFileReader.java#L49
There was a problem hiding this comment.
There are 2 init calls here so we need to handle failures from any of the two and close the reader
| // If trailer is missing or corrupt, create reader without trailer validation | ||
| LOG.warn("Invalid Trailer for file {}", filePath, invalidLogTrailerException); | ||
| // close the reader first to avoid leaking socket connection | ||
| logFileReader.close(); |
There was a problem hiding this comment.
Unfortunately, this still doesn't completely fix the issue. While it fixes the leak, you simply cannot use the same LogFileReader object because it internally maintains a closed state and then when you try to iterate it will not return any result. https://github.com/apache/phoenix/blob/PHOENIX-7562-feature-new/phoenix-core-server/src/main/java/org/apache/phoenix/replication/log/LogFileReader.java#L73-L74
You have to construct a new LogFileReader object. This also means you need to enhance your test to correctly capture the issue.
| closeReader(logFileReader); | ||
| logFileReaderContext.setValidateTrailer(false); | ||
| LogFileReader newLogFileReader = new LogFileReader(); | ||
| newLogFileReader.init(logFileReaderContext); |
There was a problem hiding this comment.
Now we are leaking the reference newLogFileReader. Maybe don't introduce a new reference and reassign the existing reference logFileReader to the new object.
2 participants
No description provided.